Add SimulateCall library #6290
Add SimulateCall library #6290
Conversation
🦋 Changeset detectedLatest commit: ed49f87 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis pull request introduces SimulateCall, a new Solidity utility library that enables simulation of external contract calls without persisting state changes through a deterministic create2-based simulator deployment. The RelayedCall utility is refactored to use named return values across all overloads. CallReceiverMock.mockFunctionExtra is updated to return address and uint256 values. Test coverage is added for SimulateCall functionality, RelayedCall tests are updated to account for value transfers, and documentation for the new utility is added. The OpenZeppelin Solidity package is bumped to a minor version. Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@test/utils/RelayedCall.test.js`:
- Around line 119-158: In the "relayed reverting call" tests update the weak
negative balance assertion to an explicit positive assertion: replace the
.to.not.changeEtherBalances([this.mock, this.revertingRelayer, this.receiver],
[0n, 0n, value]) checks with .to.changeEtherBalances([this.mock,
this.revertingRelayer, this.receiver], [-value, 0n, value]) so the test asserts
the mock paid value, the relayer balance stayed the same, and the receiver got
value; apply the same replacement for the second occurrence later in the file
(both places_use the same this.mock.$relayRevertingCall transaction and the
local value variable).
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/flat-flies-hear.mdcontracts/mocks/AccessManagedTarget.solcontracts/utils/RelayedCall.soltest/utils/RelayedCall.test.js
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: ernestognw
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/crosschain/README.adoc:1-1
Timestamp: 2025-08-28T15:48:30.716Z
Learning: ernestognw prefers "Cross chain" without hyphenation rather than "Cross-chain" in documentation titles.
📚 Learning: 2025-08-29T13:16:08.640Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/mocks/crosschain/ERC7786RecipientMock.sol:12-14
Timestamp: 2025-08-29T13:16:08.640Z
Learning: In OpenZeppelin contracts, mock contracts (like ERC7786RecipientMock) don't require input validation such as zero-address checks in constructors, as they are only used for testing purposes in controlled environments.
Applied to files:
test/utils/RelayedCall.test.js
📚 Learning: 2025-10-15T02:52:05.027Z
Learnt from: ernestognw
Repo: OpenZeppelin/openzeppelin-contracts PR: 5891
File: test/account/modules/ERC7579Module.behavior.js:56-61
Timestamp: 2025-10-15T02:52:05.027Z
Learning: In ERC7579 validator tests for `isValidSignatureWithSender`, using `this.mock` (not bound to a specific account) is valid when testing signature validation with any arbitrary sender, while `this.mockFromAccount` is used when testing account-specific validation scenarios.
Applied to files:
test/utils/RelayedCall.test.js
🧬 Code graph analysis (1)
test/utils/RelayedCall.test.js (1)
test/helpers/account.js (1)
impersonate(7-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: slither
- GitHub Check: tests
- GitHub Check: tests-foundry
- GitHub Check: tests-upgradeable
- GitHub Check: coverage
- GitHub Check: halmos
🔇 Additional comments (12)
contracts/utils/RelayedCall.sol (3)
6-53: LGTM.
144-186: LGTM.
188-274: LGTM..changeset/flat-flies-hear.md (1)
1-5: LGTM.contracts/mocks/AccessManagedTarget.sol (1)
17-19: LGTM.test/utils/RelayedCall.test.js (7)
22-38: LGTM.
46-78: LGTM.
80-116: LGTM.
170-225: LGTM.
228-265: LGTM.
267-307: LGTM.
365-420: LGTM.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
Overall, I think this is all good. Still I'd like to have a discussion about the naming. IMO reverting is an internal mechanism used to achieve a particular behavior. I would describe that behavior as "simualte a call without executing it". I believe the name should describe that behavior, and not how we implement it. That is why I would propose something like |
initcode size is smaller, makking for a smaller memory expansion.
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
|
@coderabbitai please review |
|
I see there's an ongoing discussion about naming conventions - whether 🧠 Learnings used✅ Actions performedReview triggered.
|
3 participants
Required for a version without side effects of #6003
PR Checklist
npx changeset add)